Skip to content

Conversation

@jonathanpallant
Copy link
Member

This reverts commit bd92963, reversing changes made to f475313.

Basically it switches us back to nrf-hal, until #240 is resolved.

@cloudflare-workers-and-pages
Copy link

Deploying ferrous-systems-rust-exercises with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0cee38f
Status: ✅  Deploy successful!
Preview URL: https://0841134e.ferrous-systems-rust-exercises.pages.dev
Branch Preview URL: https://revert-226-return-to-nrf-hal.ferrous-systems-rust-exercises.pages.dev

View logs

Copy link

@mhatzl mhatzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert would look good.
some diffs to #226 are not 100% and left comments for them

but since #242 fixes the new hal, this PR will likely be closed anyways

Comment on lines 84 to 85
let bmrequesttype = usbd::bmrequesttype(usbd);
let brequest = usbd::brequest(usbd);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I look at the diff in #226, the code should be

Suggested change
let bmrequesttype = usbd::bmrequesttype(usbd);
let brequest = usbd::brequest(usbd);
let bmrequesttype = usbd.bmrequesttype.read().bits() as u8;
let brequest = usbd.brequest.read().brequest().bits();

but maybe the diff view is playing tricks on me here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using the library function is better, so I didn't revert it

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diffs are not 100% flipped to #226 here, but is only formatting and the #[allow(unreachable_patterns)] is not present here, so it was likely removed since in another PR

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as usb-3

Comment on lines 83 to 84
let bmrequesttype = usbd::bmrequesttype(usbd);
let brequest = usbd::brequest(usbd);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again not exactly flipped to #226

Suggested change
let bmrequesttype = usbd::bmrequesttype(usbd);
let brequest = usbd::brequest(usbd);
let bmrequesttype = usbd.bmrequesttype.read().bits() as u8;
let brequest = usbd.brequest.read().brequest().bits();

Comment on lines 83 to 84
let bmrequesttype = usbd::bmrequesttype(usbd);
let brequest = usbd::brequest(usbd);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let bmrequesttype = usbd::bmrequesttype(usbd);
let brequest = usbd::brequest(usbd);
let bmrequesttype = usbd.bmrequesttype.read().bits() as u8;
let brequest = usbd.brequest.read().brequest().bits();

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lines 62-63 should be

let bmrequesttype = usbd.bmrequesttype().read().0 as u8;
let brequest = usbd.brequest().read().brequest().to_bits();

the request match differs but only in format and not in logic, so is fine.

[dependencies]
cortex-m = {version = "0.7.7", features = ["critical-section-single-core"]}
cortex-m-rt = "0.7.5"
dk = { path = "../boards/dk-solution", features = ["radio"] }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is correctly reverted, but not sure if you want that in this case

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a bug that the hal-app points to the solution DK.

@jonathanpallant
Copy link
Member Author

Decided not to revert - fixed instead in #242

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants